Updating FasterRCNN to use Task API#2012
Updating FasterRCNN to use Task API#2012ariG23498 wants to merge 50 commits intokeras-team:masterfrom
Conversation
ianstenbit
left a comment
ianstenbit
left a comment
There was a problem hiding this comment.
Awesome start! Definitely will want to add presets as well. I'd expect we can just drop this model into our VOC OD training script and let it rip -- should be a good way to verify numerics!
|
Relevant issue: #1960 |
|
@ianstenbit ready for another round of review. |
ianstenbit
left a comment
ianstenbit
left a comment
There was a problem hiding this comment.
Looks very close -- but we still need tests!
The linter is also unhappy:
Copyright not found in keras_cv/models/object_detection/faster_rcnn/faster_rcnn_test.py
|
@ianstenbit I am ready for another set of reviews. I have also added the tests (these are the same as that of the legacy code). |
ianstenbit
left a comment
ianstenbit
left a comment
There was a problem hiding this comment.
Awesome progress!
It would be great to plug in FasterRCNN do this training script on Colab once we think it's ready to verify that it's training correctly.
divyashreepathihalli
left a comment
divyashreepathihalli
left a comment
There was a problem hiding this comment.
Awesome Aritra! I just have one more comment.
divyashreepathihalli
left a comment
divyashreepathihalli
left a comment
There was a problem hiding this comment.
Thank you I was giving it a once-over and noticed 2 missing tests
lets us add (adding reference from yolo_v8 here)
| from keras_cv.backend import keras | ||
| from keras_cv.backend import ops | ||
|
|
||
| # from keras_cv.models.backbones.test_backbone_presets import ( |
There was a problem hiding this comment.
Hey @ariG23498! maybe you missed this part. Please add the test_backbone_presets - an equivalent one in Yolo_v8 is here
| boxes = y["boxes"] | ||
| if len(y["classes"].shape) != 2: | ||
| raise ValueError( | ||
| "Expected 'classes' to be a tf.Tensor of rank 2. " |
There was a problem hiding this comment.
update tf.Tensor to Tensor
| ((2, 512, 512, 3),), | ||
| ((2, 128, 128, 3),), | ||
| ) | ||
| def test_faster_rcnn_train(self, batch_shape): |
There was a problem hiding this comment.
lets add a model.fit() to test training
| from keras_cv.bounding_box.converters import _decode_deltas_to_boxes | ||
|
|
||
| # from keras_cv.models.backbones.backbone_presets import backbone_presets | ||
| # from keras_cv.models.backbones.backbone_presets import ( |
There was a problem hiding this comment.
remove commented imports
non max supression padded api from tf is yet to be ported
|
@ariG23498 one additional overhead work is needed. please add PS: we will fix this overhead soon, but in the mean time this is what we need to do to make sure the large GPU tests run. |
5 participants
CC: @ianstenbit